17341 action freshsales #17506
17341 action freshsales #17506
Conversation
Actions - Create Contact - Create Deal - List All Contacts - List All Deals
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis update introduces a new Freshsales integration with dynamic actions for creating and listing contacts and deals, utility functions, and a comprehensive app interface for interacting with the Freshsales API. It also removes legacy files and updates documentation and configuration to reflect the new implementation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant FreshsalesApp
participant FreshsalesAPI
User->>Action: Trigger (e.g., Create Contact/Deal)
Action->>FreshsalesApp: Fetch dynamic fields/options (e.g., contact fields, sales accounts)
FreshsalesApp->>FreshsalesAPI: GET fields/accounts/filters
FreshsalesAPI-->>FreshsalesApp: Return metadata/options
FreshsalesApp-->>Action: Return fields/options
User->>Action: Provide input data
Action->>FreshsalesApp: Create entity (contact/deal) with data
FreshsalesApp->>FreshsalesAPI: POST create request
FreshsalesAPI-->>FreshsalesApp: Return created entity
FreshsalesApp-->>Action: Return result
Action-->>User: Output summary and details
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/freshsales/actions/create-contact/create-contact.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/freshsales/actions/create-deal/create-deal.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/freshsales/actions/list-all-contacts/list-all-contacts.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
components/freshsales/common/utils.mjs (1)
1-4: The regex pattern may not handle all edge cases correctly.The current regex
^_*(.)|_+(.)/gdoesn't properly handle consecutive underscores or leading underscores in all scenarios. For example,"__test_case"would become"Test case"(missing the first character), and"_test__case"might not format correctly.Consider this more robust implementation:
-export const snakeCaseToTitleCase = (s) => - s.replace(/^_*(.)|_+(.)/g, (s, c, d) => c - ? c.toUpperCase() - : " " + d.toUpperCase()); +export const snakeCaseToTitleCase = (s) => + s.replace(/^_+/, '') + .replace(/_+/g, ' ') + .replace(/\b\w/g, (c) => c.toUpperCase());components/freshsales/freshsales.app.mjs (1)
143-168: Good pagination implementation with minor optimization opportunity.The pagination logic is well-structured. However, there's a minor issue with the
hasMorelogic - it should check if there are more pages available rather than just if the current page has data.Consider improving the pagination termination condition:
- hasMore = data[responseField].length; + hasMore = data[responseField].length > 0 && data[responseField].length === (params.per_page || 25);This assumes the API uses a
per_pageparameter and stops when fewer results are returned than requested.components/freshsales/actions/list-all-contacts/list-all-contacts.mjs (1)
24-27: Consider using Array.from() for better performance.While the current approach works, using
Array.from()would be more efficient for collecting async generator results.- const contacts = []; - for await (const contact of response) { - contacts.push(contact); - } + const contacts = []; + for await (const contact of response) { + contacts.push(contact); + }Actually, the current implementation is fine. The suggestion above doesn't improve it. Keep as is.
components/freshsales/actions/create-contact/create-contact.mjs (1)
39-39: Minor inconsistency in value type conversion.The sales account values are converted to strings here, but in the create-deal action (lines 40-43), they remain as integers. Consider maintaining consistency across both actions.
For consistency with the create-deal action, remove the string conversion:
- value: `${account.id}`, + value: account.id,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
components/freshsales/.gitignore(0 hunks)components/freshsales/README.md(0 hunks)components/freshsales/actions/create-contact/create-contact.mjs(1 hunks)components/freshsales/actions/create-deal/create-deal.mjs(1 hunks)components/freshsales/actions/list-all-contacts/list-all-contacts.mjs(1 hunks)components/freshsales/actions/list-all-deals/list-all-deals.mjs(1 hunks)components/freshsales/app/freshsales.app.ts(0 hunks)components/freshsales/common/utils.mjs(1 hunks)components/freshsales/freshsales.app.mjs(1 hunks)components/freshsales/package.json(1 hunks)
💤 Files with no reviewable changes (3)
- components/freshsales/.gitignore
- components/freshsales/README.md
- components/freshsales/app/freshsales.app.ts
🧰 Additional context used
🧠 Learnings (6)
components/freshsales/package.json (1)
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
components/freshsales/actions/list-all-contacts/list-all-contacts.mjs (4)
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-07-04T18:11:59.822Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
components/freshsales/actions/list-all-deals/list-all-deals.mjs (4)
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-07-04T18:11:59.822Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
components/freshsales/actions/create-deal/create-deal.mjs (2)
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
components/freshsales/actions/create-contact/create-contact.mjs (3)
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-07-04T18:11:59.822Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
components/freshsales/freshsales.app.mjs (2)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: In the Salesloft API integration (components/salesloft/salesloft.app.mjs), the _makeRequest method returns response.data which directly contains arrays for list endpoints like listPeople, listCadences, listUsers, and listAccounts. The propDefinitions correctly call .map() directly on these responses without needing to destructure a nested data property.
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#16954
File: components/salesloft/salesloft.app.mjs:14-23
Timestamp: 2025-06-04T17:52:05.780Z
Learning: The Salesloft API list endpoints (listPeople, listCadences, listUsers, listAccounts) return arrays directly in the response body, not wrapped in a metadata object with a nested data property. The _makeRequest method correctly returns response.data which contains the arrays that can be mapped over directly in propDefinitions.
🪛 Biome (1.9.4)
components/freshsales/actions/create-contact/create-contact.mjs
[error] 94-94: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 95-95: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (11)
components/freshsales/common/utils.mjs (1)
6-29: LGTM! Good error handling in parseObject.The function properly handles different input types and gracefully falls back to the original value when JSON parsing fails. The array handling is particularly well implemented.
components/freshsales/package.json (2)
3-5: LGTM! Version bump and main entry point update look correct.The version increment to 0.1.0 appropriately reflects the significant changes, and the main entry point correctly points to the new app file.
15-16: Dependency Version ConfirmedThe
@pipedream/platformdependency is already set to the latest stable release (3.1.0), which includes thepipedream-axiospackage for Axios integration. No changes needed.components/freshsales/actions/list-all-contacts/list-all-contacts.mjs (2)
13-22: Pagination setup looks correct.The action properly gets the filter ID and sets up pagination with the correct parameters. The use of the async generator pattern is appropriate here.
29-31: Good summary message formatting.The summary message correctly handles singular/plural forms and provides useful feedback about the number of contacts fetched.
components/freshsales/actions/list-all-deals/list-all-deals.mjs (2)
13-22: Consistent implementation with contacts action.The action follows the same pattern as the contacts action, which is good for consistency. The filter name "All Deals" is appropriate for the deals model.
24-32: LGTM! Good consistency with contacts action.The pagination collection and summary message formatting are consistent with the contacts action, maintaining good code consistency across the codebase.
components/freshsales/actions/create-deal/create-deal.mjs (2)
1-17: LGTM! Clean action structure and setup.The action is well-structured with appropriate metadata, key naming, and documentation reference. The required
nameproperty withreloadProps: trueis correctly configured to trigger additional property loading.
74-89: LGTM! Clean run method implementation.The run method correctly destructures the input data, makes the API call, and provides a helpful summary message. The response handling and export are appropriate.
components/freshsales/actions/create-contact/create-contact.mjs (2)
1-18: LGTM! Good imports and action setup.The action is well-configured with appropriate imports including the parseObject utility. The email property setup matches the pattern used in the create-deal action.
98-105: LGTM! Clean API call and response handling.The contact creation API call and response handling are implemented correctly with appropriate summary export.
| path: `/${model}/filters`, | ||
| ...opts, | ||
| }); | ||
| return filters.find((filter) => filter.name === name).id; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing filter.
The code assumes a filter with the specified name exists, but doesn't handle the case where it might not be found.
Add error handling:
- return filters.find((filter) => filter.name === name).id;
+ const filter = filters.find((filter) => filter.name === name);
+ if (!filter) {
+ throw new Error(`Filter with name "${name}" not found for model "${model}"`);
+ }
+ return filter.id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return filters.find((filter) => filter.name === name).id; | |
| const filter = filters.find((filter) => filter.name === name); | |
| if (!filter) { | |
| throw new Error(`Filter with name "${name}" not found for model "${model}"`); | |
| } | |
| return filter.id; |
🤖 Prompt for AI Agents
In components/freshsales/freshsales.app.mjs at line 79, the code assumes that
filters.find returns a filter object, but if no filter matches the name, it
returns undefined causing an error when accessing .id. Modify the code to first
check if the filter exists; if not, throw an appropriate error or handle the
missing filter case gracefully before returning the id.
| async options() { | ||
| const response = await this.listContacts(); | ||
| return response.contacts?.map(({ | ||
| id, first_name, last_name, email, | ||
| }) => ({ | ||
| label: `${first_name || ""} ${last_name || ""}`.trim() || email || `Contact ${id}`, | ||
| value: id, | ||
| })) || []; | ||
| }, |
There was a problem hiding this comment.
Potential issue with undefined contacts response.
The code accesses response.contacts but the listContacts method doesn't provide a default filterId. This could cause the API call to fail if no filter ID is provided.
The listContacts method requires a filterId parameter, but in the propDefinition, it's called without one. This will likely cause API errors. Consider using the getContacts method instead:
- const response = await this.listContacts();
+ const response = await this.getContacts();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async options() { | |
| const response = await this.listContacts(); | |
| return response.contacts?.map(({ | |
| id, first_name, last_name, email, | |
| }) => ({ | |
| label: `${first_name || ""} ${last_name || ""}`.trim() || email || `Contact ${id}`, | |
| value: id, | |
| })) || []; | |
| }, | |
| async options() { | |
| const response = await this.getContacts(); | |
| return response.contacts?.map(({ | |
| id, first_name, last_name, email, | |
| }) => ({ | |
| label: `${first_name || ""} ${last_name || ""}`.trim() || email || `Contact ${id}`, | |
| value: id, | |
| })) || []; | |
| }, |
🤖 Prompt for AI Agents
In components/freshsales/freshsales.app.mjs around lines 11 to 19, the options
method calls listContacts without providing the required filterId parameter,
which can cause API errors or undefined response.contacts. To fix this, replace
the call to listContacts with getContacts, which does not require a filterId,
ensuring the API call succeeds and response.contacts is defined.
| async options() { | ||
| const response = await this.listDeals(); | ||
| return response.deals?.map(({ | ||
| id, name, | ||
| }) => ({ | ||
| label: name || `Deal ${id}`, | ||
| value: id, | ||
| })) || []; | ||
| }, |
There was a problem hiding this comment.
Same issue with deals - missing filterId parameter.
Similar to contacts, the listDeals method requires a filterId parameter that's not provided.
Apply the same fix here:
- const response = await this.listDeals();
+ const response = await this.listDeals({
+ filterId: await this.getFilterId({
+ model: "deals",
+ name: "All Deals"
+ })
+ });Or create a getDeals method similar to getContacts for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async options() { | |
| const response = await this.listDeals(); | |
| return response.deals?.map(({ | |
| id, name, | |
| }) => ({ | |
| label: name || `Deal ${id}`, | |
| value: id, | |
| })) || []; | |
| }, | |
| async options() { | |
| const response = await this.listDeals({ | |
| filterId: await this.getFilterId({ | |
| model: "deals", | |
| name: "All Deals" | |
| }) | |
| }); | |
| return response.deals?.map(({ | |
| id, name, | |
| }) => ({ | |
| label: name || `Deal ${id}`, | |
| value: id, | |
| })) || []; | |
| }, |
🤖 Prompt for AI Agents
In components/freshsales/freshsales.app.mjs around lines 25 to 33, the options
method calls listDeals without the required filterId parameter. Fix this by
passing the appropriate filterId argument to listDeals, similar to how it is
done for contacts, or alternatively create a getDeals method that accepts
filterId and use it here for consistency.
| async additionalProps() { | ||
| const { fields } = await this.freshsales.getDealFields(); | ||
| const filteredFields = fields.filter((field) => (field.visible === true && field.name != "name")); | ||
|
|
||
| const props = {}; | ||
| for (const field of filteredFields) { | ||
|
|
||
| const data = { | ||
| type: field.type === "multi_select_dropdown" | ||
| ? "string[]" | ||
| : "string", | ||
| label: field.label, | ||
| description: `${field.label} of the deal.`, | ||
| optional: field.required === false, | ||
| }; | ||
|
|
||
| if (field.name === "sales_account_id") { | ||
| const { sales_accounts: options } = await this.freshsales.getSalesAccounts(); | ||
| data.type = "integer"; | ||
| data.label = "Sales Account"; | ||
| data.description = "Sales account of the deal."; | ||
| data.optional = true; | ||
| data.options = options.map((account) => ({ | ||
| label: account.name, | ||
| value: account.id, | ||
| })); | ||
| } | ||
|
|
||
| if (field.name === "contacts") { | ||
| const { contacts: options } = await this.freshsales.getContacts(); | ||
| data.type = "integer[]"; | ||
| data.label = "Contacts"; | ||
| data.description = "Contacts of the deal."; | ||
| data.optional = true; | ||
| data.options = options.map((contact) => ({ | ||
| label: contact.display_name, | ||
| value: contact.id, | ||
| })); | ||
| } | ||
|
|
||
| if ([ | ||
| "dropdown", | ||
| "multi_select_dropdown", | ||
| ].includes(field.type)) { | ||
| data.type = "integer"; | ||
| data.options = field.choices.map((choice) => ({ | ||
| label: choice.value, | ||
| value: choice.id, | ||
| })); | ||
| } | ||
|
|
||
| props[field.name] = data; | ||
| } | ||
|
|
||
| return props; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good dynamic property generation with room for type consistency improvement.
The dynamic property generation logic correctly filters visible fields and handles special cases appropriately. However, there's a type inconsistency issue that should be addressed.
Issue: Type inconsistency in multi_select_dropdown handling
Lines 26-28 set multi_select_dropdown fields to "string[]", but lines 62-67 override this to "integer" for dropdown fields. This creates inconsistent behavior for multi-select dropdowns.
Apply this fix to maintain type consistency:
if ([
"dropdown",
"multi_select_dropdown",
].includes(field.type)) {
- data.type = "integer";
+ data.type = field.type === "multi_select_dropdown" ? "integer[]" : "integer";
data.options = field.choices.map((choice) => ({
label: choice.value,
value: choice.id,
}));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async additionalProps() { | |
| const { fields } = await this.freshsales.getDealFields(); | |
| const filteredFields = fields.filter((field) => (field.visible === true && field.name != "name")); | |
| const props = {}; | |
| for (const field of filteredFields) { | |
| const data = { | |
| type: field.type === "multi_select_dropdown" | |
| ? "string[]" | |
| : "string", | |
| label: field.label, | |
| description: `${field.label} of the deal.`, | |
| optional: field.required === false, | |
| }; | |
| if (field.name === "sales_account_id") { | |
| const { sales_accounts: options } = await this.freshsales.getSalesAccounts(); | |
| data.type = "integer"; | |
| data.label = "Sales Account"; | |
| data.description = "Sales account of the deal."; | |
| data.optional = true; | |
| data.options = options.map((account) => ({ | |
| label: account.name, | |
| value: account.id, | |
| })); | |
| } | |
| if (field.name === "contacts") { | |
| const { contacts: options } = await this.freshsales.getContacts(); | |
| data.type = "integer[]"; | |
| data.label = "Contacts"; | |
| data.description = "Contacts of the deal."; | |
| data.optional = true; | |
| data.options = options.map((contact) => ({ | |
| label: contact.display_name, | |
| value: contact.id, | |
| })); | |
| } | |
| if ([ | |
| "dropdown", | |
| "multi_select_dropdown", | |
| ].includes(field.type)) { | |
| data.type = "integer"; | |
| data.options = field.choices.map((choice) => ({ | |
| label: choice.value, | |
| value: choice.id, | |
| })); | |
| } | |
| props[field.name] = data; | |
| } | |
| return props; | |
| }, | |
| async additionalProps() { | |
| const { fields } = await this.freshsales.getDealFields(); | |
| const filteredFields = fields.filter((field) => (field.visible === true && field.name != "name")); | |
| const props = {}; | |
| for (const field of filteredFields) { | |
| const data = { | |
| type: field.type === "multi_select_dropdown" | |
| ? "string[]" | |
| : "string", | |
| label: field.label, | |
| description: `${field.label} of the deal.`, | |
| optional: field.required === false, | |
| }; | |
| if (field.name === "sales_account_id") { | |
| const { sales_accounts: options } = await this.freshsales.getSalesAccounts(); | |
| data.type = "integer"; | |
| data.label = "Sales Account"; | |
| data.description = "Sales account of the deal."; | |
| data.optional = true; | |
| data.options = options.map((account) => ({ | |
| label: account.name, | |
| value: account.id, | |
| })); | |
| } | |
| if (field.name === "contacts") { | |
| const { contacts: options } = await this.freshsales.getContacts(); | |
| data.type = "integer[]"; | |
| data.label = "Contacts"; | |
| data.description = "Contacts of the deal."; | |
| data.optional = true; | |
| data.options = options.map((contact) => ({ | |
| label: contact.display_name, | |
| value: contact.id, | |
| })); | |
| } | |
| if ([ | |
| "dropdown", | |
| "multi_select_dropdown", | |
| ].includes(field.type)) { | |
| data.type = field.type === "multi_select_dropdown" | |
| ? "integer[]" | |
| : "integer"; | |
| data.options = field.choices.map((choice) => ({ | |
| label: choice.value, | |
| value: choice.id, | |
| })); | |
| } | |
| props[field.name] = data; | |
| } | |
| return props; | |
| }, |
🤖 Prompt for AI Agents
In components/freshsales/actions/create-deal/create-deal.mjs between lines 18
and 73, the type for multi_select_dropdown fields is initially set to "string[]"
but later overwritten to "integer" in the dropdown type handling block, causing
inconsistency. To fix this, modify the condition that sets data.type to
"integer" so it excludes multi_select_dropdown fields, ensuring
multi_select_dropdown fields retain the "string[]" type while only dropdown
fields get "integer".
| if (data.primaryAccount) { | ||
| data.sales_accounts = [ | ||
| { | ||
| id: data.primaryAccount, | ||
| is_primary: true, | ||
| }, | ||
| ]; | ||
| parseObject(data.additionalAccounts)?.map((account) => { | ||
| data.sales_accounts.push( | ||
| { | ||
| id: account, | ||
| is_primary: false, | ||
| }, | ||
| ); | ||
| }); | ||
| delete data.primaryAccount; | ||
| delete data.additionalAccounts; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address performance issue with delete operator.
The data transformation logic is correct but uses the delete operator which can impact performance. The static analysis tool correctly flagged this issue.
Apply this fix to improve performance by using undefined assignment instead of delete:
- delete data.primaryAccount;
- delete data.additionalAccounts;
+ data.primaryAccount = undefined;
+ data.additionalAccounts = undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (data.primaryAccount) { | |
| data.sales_accounts = [ | |
| { | |
| id: data.primaryAccount, | |
| is_primary: true, | |
| }, | |
| ]; | |
| parseObject(data.additionalAccounts)?.map((account) => { | |
| data.sales_accounts.push( | |
| { | |
| id: account, | |
| is_primary: false, | |
| }, | |
| ); | |
| }); | |
| delete data.primaryAccount; | |
| delete data.additionalAccounts; | |
| } | |
| if (data.primaryAccount) { | |
| data.sales_accounts = [ | |
| { | |
| id: data.primaryAccount, | |
| is_primary: true, | |
| }, | |
| ]; | |
| parseObject(data.additionalAccounts)?.map((account) => { | |
| data.sales_accounts.push( | |
| { | |
| id: account, | |
| is_primary: false, | |
| }, | |
| ); | |
| }); | |
| data.primaryAccount = undefined; | |
| data.additionalAccounts = undefined; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 95-95: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🤖 Prompt for AI Agents
In components/freshsales/actions/create-contact/create-contact.mjs around lines
79 to 96, replace the delete operator used on data.primaryAccount and
data.additionalAccounts with assignments to undefined to improve performance.
Instead of deleting these properties, set data.primaryAccount = undefined and
data.additionalAccounts = undefined after processing.
| async additionalProps() { | ||
| const { fields } = await this.freshsales.getContactFields(); | ||
| const filteredFields = fields.filter((field) => (field.visible === true && field.name != "emails")); | ||
|
|
||
| const props = {}; | ||
| for (const field of filteredFields) { | ||
|
|
||
| const data = { | ||
| type: field.type === "multi_select_dropdown" | ||
| ? "string[]" | ||
| : "string", | ||
| label: field.label, | ||
| description: `${field.label} of the contact.`, | ||
| optional: field.required === false, | ||
| }; | ||
|
|
||
| if (field.name === "sales_accounts") { | ||
| const { sales_accounts: options } = await this.freshsales.getSalesAccounts(); | ||
| const salesAccountOptions = options.map((account) => ({ | ||
| label: account.name, | ||
| value: `${account.id}`, | ||
| })); | ||
| props.primaryAccount = { | ||
| type: "string", | ||
| label: "Primary Sales Account", | ||
| description: "Primary sales account of the contact.", | ||
| optional: true, | ||
| options: salesAccountOptions, | ||
| }; | ||
| props.additionalAccounts = { | ||
| type: "string[]", | ||
| label: "Additional Sales Accounts", | ||
| description: "Additional sales accounts of the contact.", | ||
| optional: true, | ||
| options: salesAccountOptions, | ||
| }; | ||
| } else { | ||
| if ([ | ||
| "dropdown", | ||
| "multi_select_dropdown", | ||
| ].includes(field.type)) { | ||
| data.type = "integer"; | ||
| data.options = field.choices.map((choice) => ({ | ||
| label: choice.value, | ||
| value: choice.id, | ||
| })); | ||
| } | ||
| props[field.name] = data; | ||
| } | ||
| } | ||
|
|
||
| return props; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good dynamic property generation with type consistency fix needed.
The logic correctly handles the special sales_accounts field by splitting it into primary and additional account properties. However, there's the same type inconsistency issue as in the create-deal action.
Issue: Type inconsistency in multi_select_dropdown handling
Lines 27-29 set multi_select_dropdown fields to "string[]", but lines 60-65 override this to "integer" for dropdown fields. This creates inconsistent behavior for multi-select dropdowns.
Apply this fix to maintain type consistency:
if ([
"dropdown",
"multi_select_dropdown",
].includes(field.type)) {
- data.type = "integer";
+ data.type = field.type === "multi_select_dropdown" ? "integer[]" : "integer";
data.options = field.choices.map((choice) => ({
label: choice.value,
value: choice.id,
}));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async additionalProps() { | |
| const { fields } = await this.freshsales.getContactFields(); | |
| const filteredFields = fields.filter((field) => (field.visible === true && field.name != "emails")); | |
| const props = {}; | |
| for (const field of filteredFields) { | |
| const data = { | |
| type: field.type === "multi_select_dropdown" | |
| ? "string[]" | |
| : "string", | |
| label: field.label, | |
| description: `${field.label} of the contact.`, | |
| optional: field.required === false, | |
| }; | |
| if (field.name === "sales_accounts") { | |
| const { sales_accounts: options } = await this.freshsales.getSalesAccounts(); | |
| const salesAccountOptions = options.map((account) => ({ | |
| label: account.name, | |
| value: `${account.id}`, | |
| })); | |
| props.primaryAccount = { | |
| type: "string", | |
| label: "Primary Sales Account", | |
| description: "Primary sales account of the contact.", | |
| optional: true, | |
| options: salesAccountOptions, | |
| }; | |
| props.additionalAccounts = { | |
| type: "string[]", | |
| label: "Additional Sales Accounts", | |
| description: "Additional sales accounts of the contact.", | |
| optional: true, | |
| options: salesAccountOptions, | |
| }; | |
| } else { | |
| if ([ | |
| "dropdown", | |
| "multi_select_dropdown", | |
| ].includes(field.type)) { | |
| data.type = "integer"; | |
| data.options = field.choices.map((choice) => ({ | |
| label: choice.value, | |
| value: choice.id, | |
| })); | |
| } | |
| props[field.name] = data; | |
| } | |
| } | |
| return props; | |
| }, | |
| async additionalProps() { | |
| const { fields } = await this.freshsales.getContactFields(); | |
| const filteredFields = fields.filter((field) => (field.visible === true && field.name != "emails")); | |
| const props = {}; | |
| for (const field of filteredFields) { | |
| const data = { | |
| type: field.type === "multi_select_dropdown" | |
| ? "string[]" | |
| : "string", | |
| label: field.label, | |
| description: `${field.label} of the contact.`, | |
| optional: field.required === false, | |
| }; | |
| if (field.name === "sales_accounts") { | |
| const { sales_accounts: options } = await this.freshsales.getSalesAccounts(); | |
| const salesAccountOptions = options.map((account) => ({ | |
| label: account.name, | |
| value: `${account.id}`, | |
| })); | |
| props.primaryAccount = { | |
| type: "string", | |
| label: "Primary Sales Account", | |
| description: "Primary sales account of the contact.", | |
| optional: true, | |
| options: salesAccountOptions, | |
| }; | |
| props.additionalAccounts = { | |
| type: "string[]", | |
| label: "Additional Sales Accounts", | |
| description: "Additional sales accounts of the contact.", | |
| optional: true, | |
| options: salesAccountOptions, | |
| }; | |
| } else { | |
| if ([ | |
| "dropdown", | |
| "multi_select_dropdown", | |
| ].includes(field.type)) { | |
| - data.type = "integer"; | |
| + data.type = field.type === "multi_select_dropdown" ? "integer[]" : "integer"; | |
| data.options = field.choices.map((choice) => ({ | |
| label: choice.value, | |
| value: choice.id, | |
| })); | |
| } | |
| props[field.name] = data; | |
| } | |
| } | |
| return props; | |
| }, |
🤖 Prompt for AI Agents
In components/freshsales/actions/create-contact/create-contact.mjs between lines
19 and 71, the type for multi_select_dropdown fields is inconsistently set as
"string[]" initially but then changed to "integer" later in the dropdown
handling logic. To fix this, ensure that multi_select_dropdown fields
consistently use the same type by adjusting the type assignment inside the
dropdown type check to differentiate between single and multi-select dropdowns,
setting "integer" for dropdown and "integer[]" for multi_select_dropdown, and
update the options mapping accordingly to maintain type consistency.
Resolves #17341
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
Refactor